Add translation endpoint to Igbo API #812
Conversation
src/routers/routerV2.ts
Outdated
| const ONE_DAY = 24 * 60 * 60 * 1000; | ||
| const REQUESTS_PER_MS_TRANSLATION = 5; | ||
|
|
||
| const translationRateLimiter: MiddleWare = rateLimit({ |
There was a problem hiding this comment.
i think we want rate limiting to prevent too many subsequent requests in period of time rather than limiting the total number of requests that can be made. the logic that's responsible for capping the total number of requests that the current user can make with the API Key lives in authorizeDeveloperUsage.
i do think we want a rate limiter in general - we can talk about that more.
also, can we have this rate limiter not be applied if the incoming request comes from our official platforms? we would want to check to see if the X-API-Key header is equal to MAIN_KEY which is in config.ts
There was a problem hiding this comment.
After our discussion, I made this ticket for us to push a fix for rate limiting in a separate PR #814
Co-authored-by: Ijemma Onwuzulike <[email protected]>
src/routers/routerV2.ts
Outdated
| // TODO: add rate limiting with upstash | ||
| // const ONE_DAY = 24 * 60 * 60 * 1000; | ||
| // const REQUESTS_PER_MS_TRANSLATION = 5; | ||
|
|
||
| // const translationRateLimiter: MiddleWare = rateLimit({ | ||
| // windowMs: ONE_DAY, | ||
| // max: REQUESTS_PER_MS_TRANSLATION, | ||
| // }); |
There was a problem hiding this comment.
you can delete this comment since you have a corresponding GitHub issue
| }); | ||
| }; | ||
|
|
||
| const getApiTypeFromRoute = (route: string): ApiType => { |
There was a problem hiding this comment.
nit: can you add a comment for this function?
| body: { igbo: 'aka' }, | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'X-API-Key': 'main_key', |
There was a problem hiding this comment.
i think we should import MAIN_KEY instead of hardcoding 'main_key'
| data: { igbo: 'aka'.repeat(100) }, | ||
| }); | ||
| await getTranslation(req, res, next); | ||
| expect(next).toHaveBeenCalled(); |
There was a problem hiding this comment.
is it possible to check the error message here too so we know which error was thrown?
There was a problem hiding this comment.
now doing that check!
| data: { igbo: '' }, | ||
| }); | ||
| await getTranslation(req, res, next); | ||
| expect(next).toHaveBeenCalled(); |
| import { IGBO_TO_ENGLISH_API, MAIN_KEY } from '../config'; | ||
|
|
||
| interface TranslationMetadata { | ||
| igbo: string; |
There was a problem hiding this comment.
I think we would want to have a schema similar to something like the following:
| igbo: string; | |
| text: string; | |
| sourceLanguageCode: LanguageEnum; | |
| destinationLanguageCode: LanguageEnum; |
LanguageEnum would include he ISO language codes, so for Igbo it would be ibo. LanguageEnum exists in our codebase right now. This way we can avoid the potential case where we need to add more fields when we want to support more languages
There was a problem hiding this comment.
I'm now validating the request body. For now, the call to the ML endpoint won't change but that's more of an implementation detail.
|
I've added the schema changes we discussed. Also some new validation for the schema values as well as new tests for the extra validation. Let me know if any more needs to be done! |
Describe your changes
This PR updates the IgboAPI with a new endpoint to do Igbo-to-English text translation.
Motivation and Context
We're constantly trying to improve our AI product offering to include more useful Igbo technology. This now includes a machine translation endpoint served by Nkọwa okwu team.
How Has This Been Tested?
Tests can be found in
src/controllers/__tests__/translation.test.ts